Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add grpc_status to the logs #2216

Merged
merged 1 commit into from
Dec 6, 2021
Merged

Add grpc_status to the logs #2216

merged 1 commit into from
Dec 6, 2021

Conversation

ciarams87
Copy link
Contributor

Proposed changes

Add new custom log field for gRPC status.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto master
  • I will ensure my PR is targeting the master branch and pulling from my branch from my own fork

@github-actions github-actions bot added documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements labels Nov 25, 2021
@ciarams87 ciarams87 force-pushed the feat/grpc-log-field branch 2 times, most recently from 203ccc0 to 9ec09ad Compare November 26, 2021 10:26
@nginx-bot nginx-bot force-pushed the feat/grpc-log-field branch 4 times, most recently from e53954f to 0e981bd Compare November 30, 2021 18:36
pleshakov
pleshakov reviewed Dec 1, 2021
View reviewed changes
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ciarams87 Please see my comments and suggestions

internal/configs/version1/nginx-plus.tmpl Outdated Show resolved Hide resolved
internal/configs/version1/nginx-plus.tmpl Outdated Show resolved Hide resolved
examples/custom-log-format/README.md Show resolved Hide resolved
tests/suite/test_virtual_server_grpc.py Outdated Show resolved Hide resolved
@nginx-bot nginx-bot force-pushed the feat/grpc-log-field branch 2 times, most recently from b503c49 to 7da432b Compare December 2, 2021 17:55
@ciarams87 ciarams87 force-pushed the feat/grpc-log-field branch 2 times, most recently from 50f6d74 to 82b0292 Compare December 2, 2021 23:34
@nginx-bot nginx-bot force-pushed the feat/grpc-log-field branch 3 times, most recently from 177a827 to a9d4647 Compare December 3, 2021 18:17
@ciarams87 ciarams87 force-pushed the feat/grpc-log-field branch from a9d4647 to 98fd68d Compare December 6, 2021 09:09
@codecov-commenter
Copy link

Codecov Report

Merging #2216 (a9d4647) into master (0b95d7a) will decrease coverage by 0.04%.
The diff coverage is n/a.

❗ Current head a9d4647 differs from pull request most recent head 98fd68d. Consider uploading reports for the commit 98fd68d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2216      +/-   ##
==========================================
- Coverage   53.33%   53.29%   -0.05%     
==========================================
  Files          43       43              
  Lines       13392    13392              
==========================================
- Hits         7143     7137       -6     
- Misses       6020     6024       +4     
- Partials      229      231       +2
Impacted Files Coverage Δ
...ternal/k8s/appprotect/app_protect_configuration.go 86.16% <0.00%> (-0.58%) ⬇️
internal/k8s/configuration.go 95.65% <0.00%> (-0.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b95d7a...98fd68d. Read the comment docs.

@ciarams87 ciarams87 merged commit 59261a9 into master Dec 6, 2021
@ciarams87 ciarams87 deleted the feat/grpc-log-field branch December 6, 2021 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soneillf5 soneillf5 soneillf5 approved these changes

@pleshakov pleshakov pleshakov approved these changes

Assignees
No one assigned
Labels
documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ciarams87 @codecov-commenter @pleshakov @soneillf5